-
Notifications
You must be signed in to change notification settings - Fork 15
Use plotly insead of graphviz with adjustment with lobster new version #514
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Use plotly insead of graphviz with adjustment with lobster new version #514
Conversation
387df9a to
90b527e
Compare
90b527e to
66d76d2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file seems to be completely new. Is it a mistake?
README.md
Outdated
|
|
||
| ``` | ||
| $ sudo apt-get install -y graphviz | ||
| $ sudo apt-get install -y plotly |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this will fail because Plotly is not an Ubuntu package. Please verify
|
|
||
| def add_arrow_label( | ||
| fig: go.Figure, start: Tuple, end: Tuple, | ||
| ctrl: Optional[Tuple] = None, text: str = "", font_size: int = 11 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add the font_size value in the constants?
| font={"size": font_size, "color": "black"}, | ||
| align="center", | ||
| bgcolor="white", | ||
| opacity=0.85, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add the opacity value in the constants?
| @@ -34,81 +34,12 @@ | |||
| height: 24px; | |||
| vertical-align: middle; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are not using this file anywhere.
| html | ||
| ) | ||
| # Normalize timestamps - replace dynamic timestamps with static placeholder | ||
| html = re.sub( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion:
Compile regex patterns at module level it is faster than recompiling on every call
| from tests_system.lobster_html_report.lobster_UI_system_test_case_base import ( | ||
| LobsterUISystemTestCaseBase) | ||
| from tests_system.asserter import Asserter | ||
| from tests_system.lobster_html_report.\ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use the same approach in all files. In the other file you are importing it. like
from tests_system.lobster_html_report.lobster_UI_system_asserter import (
LobsterUIAsserter as Asserter)
| output = "custom_data_tracing_policy.html" | ||
| input_file = self._data_directory / "custom_data_report.lobster" | ||
|
|
||
| self.output_dir = self.create_output_directory_and_copy_expected( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code is not required?
|
@TannazVhdBMWExt |
e9de4ff to
94e2438
Compare
commit 946f102 Author: Tannaz Vahidi (ext.) <[email protected]> Date: Thu Nov 6 18:21:28 2025 +0100 Adjustments to new version of lobster Update unit/system tests and solve merge conflicts commit d3c09e8 Author: Tannaz Vahidi (ext.) <[email protected]> Date: Fri Jul 25 10:43:44 2025 +0200 Add plotly diagram to lobster-html-report Replacing graphviz and dot with plotly.
57a63cf to
d35520c
Compare
requirements.bzl
Outdated
| "flask": "@flask//:lib", | ||
| "GitPython": "@gitpython//:lib", | ||
| "kaleido": "@kaleido//:lib", | ||
| "kaleido": "@kaleido_py3//:lib", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please correct this and use the newer version of kaleido
8d874ef to
fc7f54f
Compare
|
@TannazVhdBMWExt lobster/.github/workflows/docs.yml Line 51 in fc7f54f
There are also some SVGs in the documentation folder that are generated by Graphviz. Please check those as well. |
|
After debugging the reason of hanging the system_tests on windows, we figured it out: The fact that kaleido can be imported confirms it's installed, but it doesn't prevent the underlying kaleido subprocess from hanging on complex diagrams. This is a known issue with the library. The multiprocessing code with the timeout is the correct way to handle this 1.Executes the line fig.to_image(format="svg"). 2.Launch Subprocess: The plotly library doesn't render the image in your main Python process. Instead, it finds the kaleido executable file that was installed by pip and launches it as a completely separate subprocess. This executable contains its own miniature, built-in Chromium rendering engine. 3.Send Data: Your main Python script serializes your Plotly figure (fig) into a structured JSON format. It then sends this JSON data to the kaleido subprocess through its standard input (stdin). 4.Kaleido Renders: The kaleido subprocess receives the JSON. It uses its internal Chromium engine to render the figure and create the SVG image. This is the critical step where the hang occurs. For complex diagrams, this internal engine can get stuck in a loop or encounter a bug, causing it to stop processing without crashing or sending an error. 5.Python Waits: Back in your main script, the fig.to_image() function is now paused, simply waiting for the kaleido subprocess to send the finished SVG data back through its standard output (stdout). Why It Hangs Indefinitely This is why a standard try...except block in Python fails to catch the issue—no exception is ever raised in the Python code. The process is just waiting, which is not considered an error. The multiprocessing solution works because it creates a third process that acts as a "watchdog." It watches the image-generation process and forcefully terminates it if it doesn't finish within the 15-second timeout, preventing your entire application from freezing. |
37215e2 to
2c86790
Compare
839cf88 to
795daf9
Compare
Apply review
eaf327e to
5470df4
Compare
Automatically create output directories for all LOBSTER tools to prevent crashes when paths don't exist Issue: SWF-20884
…ibrary-november-nv
…ibrary-november-nv
…ibrary-november-nv
|
Hi @SurajBDeore, thanks for your review. lobster/documentation/user-manual.md Line 32 in 312f7a7
Conclusion: These |
| "PyYAML>=6.0", | ||
| "yamale>=6.0.0", | ||
| "GitPython>=3.1.30", | ||
| "plotly==5.10.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need to include kaleido?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, I think so
|
I was able to generate an HTML report using Linux. I did not test on other OS, though. |
|
I tried generating the HTML report on Windows, but it was not generated. To confirm the issue, I also tried using a different system with this PR, but the HTML report is still not working. |
|
Known issue with |
No description provided.